HTTPCLIENT-2352: Improved flow control window handling to prevent overflow/underflow#510
HTTPCLIENT-2352: Improved flow control window handling to prevent overflow/underflow#510arturobernalg wants to merge 1 commit intoapache:masterfrom
Conversation
…rflow/underflow - Updated `updateWindow` method to cap the flow control window at `Integer.MAX_VALUE` and `Integer.MIN_VALUE`, avoiding `ArithmeticException` during updates. - Modified `maximizeConnWindow` to account for the capped behavior, ensuring seamless operation within flow control limits.
| // Test non-overflow scenario for both implementations | ||
| window = new AtomicInteger(1); | ||
| final int resultNewSafe = updateWindowNew(window, Integer.MAX_VALUE / 2 - 1); | ||
| System.out.println("New safe update result: " + resultNewSafe); |
There was a problem hiding this comment.
I don't think we should write to the console.
|
@arturobernalg I am sorry but this is really not a good idea. We cannot just disable the data window overflow check and hope for the best. HTTP/2 endpoints can make mistakes in calculating their data windows. We need to detect those mistakes and report them as protocol violations |
@ok2c I see your point about not bypassing the overflow check. How about keeping the protocol violation handling by throwing an H2ConnectionException on overflow? This way, we still catch misbehaving endpoints without silently ignoring the issue. |
@arturobernalg This is hardly any different to what we have now, slightly more readable and slightly less efficient. I would like to see protocol logs with the exception and ideally a reproducer before any changes are made. |
Fair enough @ok2c |
|
Superseded by #511 |
This PR addresses issues related to the flow control window management in the HTTP/2 implementation of Apache HttpClient. Specifically, it ensures that flow control window updates do not cause
ArithmeticExceptiondue to overflow or underflow.